Skip to content

feat: use the network source architecture (defineNetwork)#2650

Open
kettanaito wants to merge 99 commits intomainfrom
fix/define-network
Open

feat: use the network source architecture (defineNetwork)#2650
kettanaito wants to merge 99 commits intomainfrom
fix/define-network

Conversation

@kettanaito
Copy link
Copy Markdown
Member

@kettanaito kettanaito commented Jan 10, 2026

Changes

  • Rewrites the way the SetupApi behaves by introducing: network frames, sources, controllers, and defineNetwork.
  • Handlers are now stored in a kind-based map. This should improve the performance during handler lookup since it becomes O(1).
  • Fixes an issue where a WebSocket connection would be logged in the console even when there are no matching event handlers for it.
  • Handlers filtering no longer uses instanceof check. Instead, the kind property of the handler is used.

Usage

The introduces APIs are meant to be used from the explicit msw/future export path. The path itself isn't documented as the APIs are considered experimental until the next major release.

Roadmap

  • Ensure the changes are backward-compatible
    • I removed HandlersKind and just inlined them in the handler classes.
    • I removed __kind private field from handler classes and made kind public. This is a safe change as nobody should be using these fields.
  • Passing handlers as both handlers and the constructor argument for the handlers controller seems odd. Choose just one?
    • Merged two into a single handlers option. After all, handlers are only needed as the input to the controller.
  • Consider higher-level utilities to create things like custom setupServer. The defineNetwork API is rather low-level. Export NetowrkOptions to the user can just override some of them?
import { defineNetwork } from 'msw'
import { defaultNetwork } from 'msw/node'

// Create a custom "setup*" wrapper.
function setupCustomApi(...handlers) {
  return defineNetwork({
    ...defaultNetwork,
    ...customizations,
    handlers,
  })
}

// Use it as you normally would use "setup*" APIs.
const network = setupCustomApi(...handlers)
await network.enable()
  • Export createSetupServerCommonApi or its alternative. That's a crucial part of extending the default behaviors.
    • I'll hold on regarding this. Let the userland decide what utilities are missing. The common SetuApi isn't that difficult to implement by hand.
  • Fix the issue where onUnhandledFrame couldn't affect the request resolution because it's called after frame.resolve() is finished.
  • Add a test for onUnhandledRequest: 'error' to ensure that the request is errored (not bypassed). There's currently a bug since onUnhandledFrame is called after frame.resolve(), which always calls passthrough() before the unhandled frame callback.
  • Add a test for multiple WebSocket handlers that match the same URL. Make the first throw an error intentionally. Make sure that the next two still fire off. I suspect due to the algo change in WebSocketNetworkFrame, that won't happen (exception in one handler will short-circuit the entire loop).
    • ⚠️ The handler lookup must short-circuit. This is how HTTP behaves, this is how WebSocket should behave to remain predictable.
  • Unit tests for the introduced functionality.
  • Check if unhandled frame logic and exception try/catch in WebSocketInterceptor aren't fighting each other. I suspect since we catch exceptions, they never surface to the interceptor and it never properly translates them to socket error + closure.
  • Consider a new API to replace server.boundary() to establish ASL boundaries for Node.js tests. The NetworkApi must remain the same across environments. Environment-specific APIs must come from the environment entrypoints (msw/node) and applied explicitly.
    • Consider AsyncHandlersController.boundary() instead of the boundary() method in setup-server.ts.
  • (Potentially unneeded) Introduce readyState to NetworkApi so implementers don't have to introduce it themselves (e.g. remove isStarted from setup-worker.ts and use a network.readyState check).
  • ⚠️ Solve the optional asynchronicity in .enable() leaking from the network sources. Not awaiting network.disable() in server.stop() feels weird.
  • Write unit tests for colorless enable/disable in defineNetwork.
  • ⚠️ SetupWorkerApi was exported as a class before, now it's only a type. Re-implement that class similar to how we re-implement SetupServerApi for backwards compatibility.

@kettanaito kettanaito changed the title feat: add the defineNetwork API feat: migrate to the defineNetwork API Jan 11, 2026
@kettanaito kettanaito changed the title feat: migrate to the defineNetwork API feat: implement the defineNetwork API Jan 14, 2026
@felmonon
Copy link
Copy Markdown
Contributor

Opened #2676 as a focused follow-up for the RequestHandler[] compatibility issue reported in testing on this PR.

It widens the shared AnyHandler type to RequestHandler | WebSocketHandler and adds regression coverage for both setupWorker and setupServer.

Verified with:

  • pnpm build
  • pnpm test:ts
  • pnpm test:unit -- --run src/core/experimental/handlers-controller.test.ts test/node/msw-api/setup-server/listHandlers.node.test.ts
  • pnpm lint

Co-authored-by: Felmon Fekadu <felmonon@gmail.com>
Co-authored-by: Artem Zakharchenko <kettanaito@gmail.com>
@kettanaito kettanaito added this to the 3.0 milestone Mar 20, 2026
@kettanaito
Copy link
Copy Markdown
Member Author

Colorless enable/disable

Problem

defineNetwork's enable/disable methods are both async and sync depending on the network sources provided. It's easier to make them always async and just ask consumers to await them (likely what we will do in the future), but at the moment, one of such consumers is setupServer, whose listen/close methods are synchronous.

Solution

  • Iterate over return values of sources.map(source => source.enable()) and if any of them returns a promise, return the chained promise from enable() on defineNetwork. The same approach for disable().
  • Use conditional return type in enable/disable that analyzes whether any of the Sources return a promise.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 3, 2026

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/define-network

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
.github/workflows/ci.yml (4)

10-11: ⚠️ Potential issue | 🟠 Major

Job build-20 now runs on Node 22, losing Node 20 build coverage.

The job is named build-20 and labeled "build (20)", but node-version is set to 22. This defeats the purpose of having separate build jobs for different Node versions. The cache key at line 41 still references node-20, which is now misleading.

If Node 20 compatibility testing is still required, revert this job to use node-version: 20. Otherwise, rename the job to build-22 and consolidate with the existing build-22 job.

🐛 Proposed fix to restore Node 20 testing
       - name: Set up Node.js
         uses: actions/setup-node@v4
         with:
-          node-version: 22
+          node-version: 20
           cache: 'pnpm'

Also applies to: 25-26

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 10 - 11, The CI job named build-20 is
configured to run on Node 22 (node-version: 22) while still using cache keys
labeled node-20, causing misleading coverage; either change node-version back to
20 in the build-20 job and update its cache keys to match, or rename the job to
build-22 (and update the job name/labels and cache key prefixes) and remove or
consolidate the duplicate build-22 job; also apply the same fix pattern to the
related jobs referenced as 25-26 so job names, node-version entries, and cache
key prefixes stay consistent.

182-199: ⚠️ Potential issue | 🟡 Minor

Job name "test (e2e) (20)" suggests Node 20 but runs on Node 22.

The test-e2e job is labeled "test (e2e) (20)" but uses node-version: 22. Either restore Node 20 for consistency with the job name or update the job name to reflect the actual Node version.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 182 - 199, The GitHub Actions job
"test-e2e" has a mismatched label vs runtime: the job's name string "test (e2e)
(20)" does not match the configured Node version (actions/setup-node
node-version: 22); update either the name to reflect Node 22 (e.g., change the
name value used in the job) or change the node-version from 22 back to 20 so the
label and runtime are consistent (refer to the "test-e2e" job, the name field
"test (e2e) (20)", and the actions/setup-node with node-version).

80-97: ⚠️ Potential issue | 🟡 Minor

test-unit depends on build-20 but runs on Node 22.

The test-unit job depends on build-20 and restores a cache keyed with node-20, but now runs on Node 22. If build-20 is fixed to use Node 20, this job should also use Node 20 for consistency, or explicitly depend on the Node 22 build.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 80 - 97, The test-unit job currently
declares needs: build-20 but sets actions/setup-node@v4 with node-version: 22,
causing a mismatch with the build that restores a node-20 cache; update the
test-unit job (job name: test-unit) to use node-version: 20 in the
actions/setup-node step to match the build-20 cache, or if you intend to run
tests on Node 22 instead, change the dependency from needs: build-20 to the
appropriate build job that ran on Node 22 and ensure the cache key/restore logic
aligns with that Node version.

116-117: ⚠️ Potential issue | 🟠 Major

Job test-node-20 now runs on Node 22, losing Node 20 test coverage.

The job is named test-node-20 and labeled "test (node.js) (20)", but the runtime is Node 22. Combined with the build job issue, this means no Node 20 testing occurs despite the job naming suggesting otherwise.

🐛 Proposed fix to restore Node 20 testing
       - name: Set up Node.js
         uses: actions/setup-node@v4
         with:
-          node-version: 22
+          node-version: 20
           cache: 'pnpm'

Also applies to: 132-133

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 116 - 117, The CI job named
test-node-20 is configured to run Node 22 despite its name/label; update the
job's Node runtime configuration (e.g., the actions/setup-node node-version or
the matrix value used by the test-node-20 job) to use node version 20 instead of
22, and make the same correction for the duplicate occurrence referenced around
the other block (the second test-node-20 occurrence at the later lines) so the
job name, label, and runtime all consistently run Node 20.
config/replaceCoreImports.js (1)

1-10: ⚠️ Potential issue | 🟡 Minor

Potential stateful regex issue with test() and the global flag.

The hasCoreImports function calls test() on a regex that has the g (global) flag. When test() is called on a global regex, it updates lastIndex, which can cause subsequent calls to the same regex to start matching from that position rather than from the beginning. This can lead to inconsistent results if the same regex is reused.

Since replaceCoreImports also uses the same regex pattern, and they share the same regex objects (CORE_ESM_IMPORT_PATTERN / CORE_CJS_IMPORT_PATTERN), calling hasCoreImports before replaceCoreImports could affect the replacement behavior.

🔧 Suggested fix: Reset lastIndex or create new regex instances
 function getCoreImportPattern(isEsm) {
-  return isEsm ? CORE_ESM_IMPORT_PATTERN : CORE_CJS_IMPORT_PATTERN
+  // Return new instances to avoid global regex state issues
+  return isEsm
+    ? /from ["'](`#core`(.*))["'](;)?/gm
+    : /require\(["'](`#core`(.*))["']\)(;)?/gm
 }

Alternatively, reset lastIndex before use:

 export function hasCoreImports(fileContents, isEsm) {
-  return getCoreImportPattern(isEsm).test(fileContents)
+  const pattern = getCoreImportPattern(isEsm)
+  pattern.lastIndex = 0
+  return pattern.test(fileContents)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@config/replaceCoreImports.js` around lines 1 - 10, hasCoreImports uses shared
regexes CORE_ESM_IMPORT_PATTERN/CORE_CJS_IMPORT_PATTERN that were created with
the global flag, so successive test() calls mutate lastIndex and cause flaky
results; fix by ensuring you use fresh regex instances or reset lastIndex before
testing/replacing: update getCoreImportPattern to return a new RegExp based on
the same pattern/flags (or explicitly set getCoreImportPattern(...).lastIndex =
0 before calling test) and apply the same approach in replaceCoreImports so both
hasCoreImports and replaceCoreImports operate on non-stateful regexes.
src/core/ws/utils/attachWebSocketLogger.ts (1)

85-96: ⚠️ Potential issue | 🟡 Minor

Missing abort signal on nested server.addEventListener('message', ...).

The inner message event listener added to server (line 88-90) does not use { signal: controller.signal }. When the cleanup function is called, this listener will remain attached, potentially causing a memory leak or stale logging.

🔧 Proposed fix
   server.addEventListener(
     'open',
     () => {
-      server.addEventListener('message', (event) => {
-        logIncomingServerMessage(event)
-      })
+      server.addEventListener(
+        'message',
+        (event) => {
+          logIncomingServerMessage(event)
+        },
+        { signal: controller.signal },
+      )
     },
     {
       once: true,
       signal: controller.signal,
     },
   )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/ws/utils/attachWebSocketLogger.ts` around lines 85 - 96, The nested
server.addEventListener('message', ...) inside the 'open' handler is missing the
abort signal so it won't be removed on cleanup; update the inner registration
(the listener that calls logIncomingServerMessage) to pass the same { signal:
controller.signal } option (or use a named handler and call
server.removeEventListener(...) on cleanup) so the message listener is
unregistered when controller.signal aborts; locate this in the block that
registers the 'open' event and modify the inner server.addEventListener call
accordingly.
🟡 Minor comments (5)
src/core/experimental/request-utils.ts-22-38 (1)

22-38: ⚠️ Potential issue | 🟡 Minor

Regex does not handle msw/passthrough appearing first in a multi-value Accept header.

The pattern /(,\s+)?msw\/passthrough/ only matches an optional comma before the token. If the Accept header is "msw/passthrough, application/json", the result will be ", application/json" with a leading comma.

Consider handling both positions:

Proposed fix
-    const nextAcceptHeader = acceptHeader.replace(/(,\s+)?msw\/passthrough/, '')
+    const nextAcceptHeader = acceptHeader
+      .replace(/msw\/passthrough,\s*/, '')
+      .replace(/(,\s+)?msw\/passthrough/, '')

Alternatively, a single regex with alternation:

-    const nextAcceptHeader = acceptHeader.replace(/(,\s+)?msw\/passthrough/, '')
+    const nextAcceptHeader = acceptHeader.replace(/(,\s*)?msw\/passthrough(,\s*)?/, (_, before, after) => (before && after ? ', ' : ''))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/request-utils.ts` around lines 22 - 38, The
accept-token removal regex in deleteRequestPassthroughHeader doesn't handle
msw/passthrough when it's the first value; update the replacement to remove the
token plus any surrounding commas/whitespace regardless of position by using a
regex like /(?:^|\s*,\s*)msw\/passthrough(?:\s*,\s*|$)/ for the replace in
deleteRequestPassthroughHeader (i.e., change how nextAcceptHeader is computed),
then normalize the resulting header string by trimming and collapsing any
leftover leading/trailing commas/extra whitespace before setting or deleting the
accept header.
src/core/experimental/on-unhandled-frame.test.ts-302-302 (1)

302-302: ⚠️ Potential issue | 🟡 Minor

Fix two test titles that reference the wrong frame type.

Line 302 and Line 332 say “HTTP frame” but both tests exercise TestWebSocketFrame. Rename to avoid confusion in CI failure output.

✏️ Proposed rename
-it('supports printing the default warning in the custom callback for the HTTP frame', async () => {
+it('supports printing the default warning in the custom callback for the WebSocket frame', async () => {
-it('supports printing the default error in the custom callback for the HTTP frame', async () => {
+it('supports printing the default error in the custom callback for the WebSocket frame', async () => {

Also applies to: 332-332

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/on-unhandled-frame.test.ts` at line 302, Two test
titles incorrectly say "HTTP frame" while exercising TestWebSocketFrame; update
the titles to reference WebSocket/TestWebSocketFrame to avoid confusion. Locate
the two it(...) blocks whose current descriptions begin with "supports printing
the default warning in the custom callback for the HTTP frame" and change them
to "supports printing the default warning in the custom callback for the
WebSocket frame (TestWebSocketFrame)" (or similar wording that includes
TestWebSocketFrame), ensuring both occurrences are updated so CI output
correctly identifies the tested frame type.
src/core/experimental/frames/websocket-frame.test.ts-182-182 (1)

182-182: ⚠️ Potential issue | 🟡 Minor

Typo in error message: "exceptin" should be "exception".

Proposed fix
-  const exception = new Error('Unhandled exceptin')
+  const exception = new Error('Unhandled exception')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/frames/websocket-frame.test.ts` at line 182, Fix the
typo in the test error message by changing the string passed to the Error
constructor for the variable named exception from "Unhandled exceptin" to
"Unhandled exception" in the websocket-frame.test; locate the declaration const
exception = new Error('Unhandled exceptin') and update the message text only so
the test error message is spelled correctly.
src/core/experimental/handlers-controller.ts-49-53 (1)

49-53: ⚠️ Potential issue | 🟡 Minor

currentHandlers() now reorders mixed handler lists.

Flattening Object.values(this.getState().handlers) groups handlers by kind, so listHandlers() no longer reflects the order passed to setup*(), use(), or resetHandlers() when HTTP and WebSocket handlers are interleaved.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/handlers-controller.ts` around lines 49 - 53,
currentHandlers() flattens handlers by kind and thus reorders mixed
HTTP/WebSocket registrations; preserve original registration order by changing
state to track insertion sequence and returning handlers in that sequence: add
an ordered list (e.g., state.orderedHandlers) that setup*(), use(), and
resetHandlers() update on add/remove, and modify currentHandlers() to iterate
state.orderedHandlers to collect non-null handlers (instead of
Object.values(...).flat()), or alternately store a registration timestamp on
each handler and sort by it in currentHandlers(); reference currentHandlers(),
setup*(), use(), and resetHandlers() when making these changes.
src/core/experimental/on-unhandled-frame.ts-18-21 (1)

18-21: ⚠️ Potential issue | 🟡 Minor

defaults.warn/error are async, not sync.

Both defaults are bound to printStrategyMessage() (line 27), an async function that awaits frame.getUnhandledMessage() (line 34). The type declaration at lines 18-21 declares them as () => void, but the actual implementation returns Promise<void>. This prevents using await defaults.warn() in custom callbacks and makes the defaults fire-and-forget by default.

Suggested fix
 export type UnhandledFrameDefaults = {
-  warn: () => void
-  error: () => void
+  warn: () => Promise<void>
+  error: () => Promise<void>
 }

Also applies to: 27-45, 84-91

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/on-unhandled-frame.ts` around lines 18 - 21, The
UnhandledFrameDefaults type and usages declare warn/error as synchronous but the
implementation (printStrategyMessage, which awaits frame.getUnhandledMessage)
returns Promise<void>; update the UnhandledFrameDefaults type so warn and error
are typed as () => Promise<void>, and adjust any places that construct or call
defaults.warn/defaults.error (e.g., where defaults are created and where they
are invoked in printStrategyMessage and other call sites around lines 27-45 and
84-91) to handle/await the returned Promise accordingly (or explicitly return
the Promise) so callers can await these handlers.
🧹 Nitpick comments (15)
test/node/rest-api/request/body/body-form-data.node.test.ts (1)

50-54: Reconsidering expect.soft() for status assertions.

Using expect.soft() for the HTTP status check means the test will continue to the JSON body assertion even if the status is wrong. If the response isn't 200, the body assertion at line 51 will likely fail with a confusing error (e.g., an error response body instead of the expected entries), masking the actual root cause.

If the intent is to see all failures, consider whether a failing status check should still proceed to body assertions, or if a hard failure is more appropriate here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/node/rest-api/request/body/body-form-data.node.test.ts` around lines 50
- 54, The status assertion uses expect.soft(res.status) which allows the test to
continue on non-200 responses and can produce misleading body-failure noise;
change the status check to a hard assertion (expect(res.status).toBe(200)) so
the test immediately fails on unexpected HTTP status and prevents subsequent
json assertions from running, updating the assertion that references res.status
in this test file (body-form-data.node.test.ts) accordingly.
test/browser/msw-api/setup-worker/scenarios/iframe-isolated-response/iframe-isolated-response.test.ts (1)

64-66: Redundant null checks after function type checks.

These != null checks are unnecessary. Lines 59-62 already wait for typeof window.request === 'function', which guarantees the value is neither null nor undefined. A function is always truthy and non-null.

🧹 Proposed fix to remove redundant checks
   await Promise.all([
     frameOne.waitForFunction(() => typeof window.request === 'function'),
     frameTwo.waitForFunction(() => typeof window.request === 'function'),
   ])
 
-  await frameOne.waitForFunction(() => window.request != null)
-  await frameTwo.waitForFunction(() => window.request != null)
-
   await Promise.all([
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@test/browser/msw-api/setup-worker/scenarios/iframe-isolated-response/iframe-isolated-response.test.ts`
around lines 64 - 66, The two redundant waits using frameOne.waitForFunction(()
=> window.request != null) and frameTwo.waitForFunction(() => window.request !=
null) should be removed because earlier waits already assert typeof
window.request === 'function' (so request cannot be null); delete these two
lines (the calls to waitForFunction that check != null) to avoid duplicate
checks and keep only the prior typeof checks that guarantee a function exists.
.github/workflows/ci.yml (2)

250-267: test-native job changed to Node 22.

Same as other jobs depending on build-20—verify alignment once the Node 20/22 split strategy is clarified.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 250 - 267, The test-native job
currently sets node-version: 22 but declares needs: build-20, causing a Node
version mismatch; update the test-native job (look for the test-native job block
and its node-version key) to align with the intended strategy—either change
node-version to 20 to match build-20 or update the dependent build job and any
other jobs to use Node 22 consistently—ensure the node-version value is
consistent across jobs that share the same build dependency.

207-224: test-browser job changed to Node 22.

This job depends on build-20 and uses cache keys referencing node-20. If Node 20 testing is restored for the build job, ensure this job's Node version and dependencies are aligned.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 207 - 224, The test-browser job now
uses Node 22 while it depends on build-20 and cache keys still reference
node-20; update the test-browser job (job name: test-browser) to align Node
version and cache with the build job by setting uses: actions/setup-node@v4 with
node-version matching the build (e.g., 20) and ensure the pnpm cache key or
related references that mention node-20 are consistent, or conversely update
build-20 and its cache keys to Node 22 if you intend to move both jobs to Node
22 so both job names and cache keys remain in sync (refer to job names
test-browser and build-20 and the cache/key entries).
src/core/experimental/request-utils.test.ts (1)

49-69: Consider adding test coverage for multi-value Accept headers.

The current tests only cover a single-value accept: 'msw/passthrough' header. Consider adding a test for when msw/passthrough appears alongside other accept values (e.g., 'application/json, msw/passthrough') to verify the header is correctly trimmed while preserving other values.

Suggested additional test
it('removes the bypass header while preserving other accept values', () => {
  const request = new Request('http://example.com', {
    headers: {
      accept: 'application/json, msw/passthrough',
    },
  })
  deleteRequestPassthroughHeader(request)
  expect(request.headers.get('accept')).toBe('application/json')
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/request-utils.test.ts` around lines 49 - 69, Add a new
unit test in src/core/experimental/request-utils.test.ts that verifies
deleteRequestPassthroughHeader correctly removes the "msw/passthrough" token
from a multi-value Accept header while preserving other values; create a Request
with headers.accept set to a comma-separated string like "application/json,
msw/passthrough", call deleteRequestPassthroughHeader(request), and assert that
request.headers.get('accept') equals "application/json" (trimmed of whitespace
as needed) to ensure multi-value handling is covered for the
deleteRequestPassthroughHeader function.
test/support/ws-test-utils.ts (2)

27-31: Type assertions may mask compatibility issues.

The as any casts on socket and transport bypass type checking. If the WebSocketServerConnection constructor signature changes, these tests won't catch the mismatch at compile time. Consider adding a brief comment explaining why the casts are necessary (e.g., test environment limitations).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/support/ws-test-utils.ts` around lines 27 - 31, The test is using unsafe
casts for socket and transport when instantiating WebSocketServerConnection
which masks type mismatches; update the instantiation around
WebSocketServerConnection to either use correctly typed test doubles/mocks for
socket and transport or, if the test environment prevents that, replace the two
"as any" casts with a short inline comment explaining why the casts are required
(e.g., test harness types differ from runtime types) and reference the variables
socket and transport so future maintainers know the intentional tradeoff; prefer
creating minimally-typed stubs matching the constructor signature over using any
when feasible.

1-9: Consolidate duplicate imports from the same module.

The two import statements from @mswjs/interceptors/WebSocket can be merged into a single statement.

Suggested consolidation
-import {
-  WebSocketClientConnection,
-  WebSocketConnectionData,
-  WebSocketServerConnection,
-} from '@mswjs/interceptors/WebSocket'
-import {
-  WebSocketTransport,
-  WebSocketData,
-} from '@mswjs/interceptors/WebSocket'
+import {
+  WebSocketClientConnection,
+  WebSocketConnectionData,
+  WebSocketServerConnection,
+  WebSocketTransport,
+  WebSocketData,
+} from '@mswjs/interceptors/WebSocket'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/support/ws-test-utils.ts` around lines 1 - 9, The file has duplicate
import statements from the same module; consolidate them into a single import
from '@mswjs/interceptors/WebSocket' that imports WebSocketClientConnection,
WebSocketConnectionData, WebSocketServerConnection, WebSocketTransport, and
WebSocketData together to remove redundancy and keep imports tidy.
src/core/ws/handleWebSocketEvent.ts (1)

20-24: Resolve this TODO by switching to kind-indexed lookup.

Line 24 still does a per-connection filter over all handlers. Since handler maps are now kind-grouped, using controller-level getHandlersByKind('websocket') here would remove repeated O(n) scans.

I can draft the exact interface + call-site patch for this file and its options wiring if you want me to open a follow-up issue.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/ws/handleWebSocketEvent.ts` around lines 20 - 24, The current code
in handleWebSocketEvent.ts builds handlers by filtering all handlers via
options.getHandlers().filter(isHandlerKind('websocket')), causing an O(n) scan
per connection; replace that with the kind-indexed lookup by calling
options.getHandlersByKind('websocket') (or the controller method exposed on
options) and assign its result to the handlers variable, removing the
isHandlerKind filter usage; ensure any call-sites expecting an array still work
and that the options object exposes getHandlersByKind so the new call returns
the pre-grouped websocket handlers.
src/core/experimental/sources/network-source.test.ts (1)

21-24: Extract repeated CustomNetworkSource test fixture.

Same inline class appears in multiple tests; a small helper keeps this suite tighter and easier to evolve.

♻️ Optional cleanup
+class TestNetworkSource extends NetworkSource {
+  enable = async () => {}
+}
+
 it('emits the "frame" event when a frame is queued', async () => {
-  class CustomNetworkSource extends NetworkSource {
-    enable = async () => {}
-  }
-
-  const source = new CustomNetworkSource()
+  const source = new TestNetworkSource()
   const frameListener = vi.fn()
   source.on('frame', frameListener)
@@
 it('removes all listeners when "disable" is called', async () => {
-  class CustomNetworkSource extends NetworkSource {
-    enable = async () => {}
-  }
-
-  const source = new CustomNetworkSource()
+  const source = new TestNetworkSource()
@@
 it('accepts AbortSignal when attaching event listeners', async () => {
-  class CustomNetworkSource extends NetworkSource {
-    enable = async () => {}
-  }
-
-  const source = new CustomNetworkSource()
+  const source = new TestNetworkSource()

Also applies to: 38-40, 53-55

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/sources/network-source.test.ts` around lines 21 - 24,
Extract the repeated inline test class CustomNetworkSource into a single shared
fixture: create a top-level helper (e.g., a function or exported test-class)
named createCustomNetworkSource or SharedCustomNetworkSource and replace each
inline class declaration (instances at CustomNetworkSource in the three test
sites) with calls to that helper; update tests that reference enable or
instantiate the class to use the shared helper so the suite doesn't duplicate
the same class definition across tests.
src/core/experimental/compat.ts (1)

27-34: Consider setting HTTP method for WebSocket upgrade request.

The synthetic Request for WebSocket frames only sets connection and upgrade headers. WebSocket upgrade requests are typically GET requests. While this may not matter for the legacy callback's use case, setting the method explicitly would be more accurate.

💡 Optional enhancement
             : frame instanceof WebSocketNetworkFrame
               ? new Request(frame.data.connection.client.url, {
+                  method: 'GET',
                   headers: {
                     connection: 'upgrade',
                     upgrade: 'websocket',
                   },
                 })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/compat.ts` around lines 27 - 34, The synthetic Request
created for WebSocket frames (the ternary branch that checks frame instanceof
WebSocketNetworkFrame and constructs new
Request(frame.data.connection.client.url, {...})) does not set an HTTP method;
make it an explicit GET by adding method: 'GET' to the Request init object so
the upgrade request accurately reflects a WebSocket handshake (keep the existing
headers: connection: 'upgrade' and upgrade: 'websocket').
test/browser/msw-api/setup-worker/stop.test.ts (1)

65-66: Clarify the purpose of expect.soft() here.

Using expect.soft() for the fromServiceWorker() assertion allows the test to continue even if this check fails. If this is intentional (e.g., the service worker behavior is secondary to the main assertion), this is fine. Otherwise, consider using a regular expect() if both assertions are equally important.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/browser/msw-api/setup-worker/stop.test.ts` around lines 65 - 66, The use
of expect.soft(response.fromServiceWorker()) is ambiguous; decide whether the
fromServiceWorker() check is critical. If it is critical, replace
expect.soft(response.fromServiceWorker()).toBe(true) with a regular
expect(response.fromServiceWorker()).toBe(true) so the test fails immediately on
mismatch; if the check is intentionally non-blocking, keep expect.soft but add a
brief inline comment above that line explaining why the assertion is allowed to
fail without aborting the test (e.g., "non-fatal check: service worker presence
is optional for this scenario"). Ensure you reference the same call sites:
expect.soft / expect and response.fromServiceWorker(), leaving the await
expect(response.json()).resolves.toEqual({ original: true }) unchanged.
test/browser/msw-api/setup-worker/start/on-unhandled-request/callback-print.test.ts (1)

70-70: Replace fixed 1s sleeps with deterministic synchronization.

Line 70 and Line 149 add unconditional delays that slow the suite and can still be flaky. Prefer waiting on a concrete signal (specific console entry/network completion) or remove these waits if assertions already cover completion.

♻️ Proposed change
-  await page.waitForTimeout(1000)
+  // No fixed delay needed; assertions above already await all expected side-effects.
-  await page.waitForTimeout(1000)
+  // No fixed delay needed; assertions above already await all expected side-effects.

Also applies to: 149-149

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@test/browser/msw-api/setup-worker/start/on-unhandled-request/callback-print.test.ts`
at line 70, Replace the unconditional page.waitForTimeout(1000) sleeps in
callback-print.test.ts (instances at the call to page.waitForTimeout) with
deterministic waits: wait for the specific console message using
page.waitForEvent('console') filtered by message text, or wait for the network
response with page.waitForResponse() that matches the request URL, or use
page.waitForFunction() to poll a DOM/test-visible state that your assertions
rely on; update both occurrences so assertions run only after these concrete
signals instead of a fixed timeout.
src/core/experimental/setup-api.ts (1)

30-37: Wire events through publicEmitter (or remove publicEmitter).

publicEmitter is created and disposed, but never used for subscriptions/emissions; events is bound directly to emitter. That makes the extra emitter dead state and weakens internal/public event separation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/setup-api.ts` around lines 30 - 37, The code creates
both emitter and publicEmitter but binds this.events to emitter so publicEmitter
is unused; either remove publicEmitter entirely or wire events through it:
replace this.events = this.emitter with this.events = this.publicEmitter and
then add forwarding so internal emits also publish to publicEmitter (e.g., in
the constructor attach a forwarder from this.emitter to this.publicEmitter by
calling this.emitter.on(event, (...args) => this.publicEmitter.emit(event,
...args)) for the relevant event names or by wrapping emitter.emit to call
publicEmitter.emit as well); ensure the cleanup in this.subscriptions still
removes listeners from both this.emitter and this.publicEmitter.
src/browser/glossary.ts (1)

5-5: Consider using a type-only import for AnyHandler.

AnyHandler is only used in type positions within this file. Using a type import (import type) would make the intent clearer and could improve tree-shaking.

Suggested fix
-import { AnyHandler } from '#core/experimental/handlers-controller'
+import type { AnyHandler } from '#core/experimental/handlers-controller'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/browser/glossary.ts` at line 5, The import of AnyHandler in glossary.ts
is used only as a type; change it to a type-only import to clarify intent and
aid tree-shaking by replacing the current import of AnyHandler with a type
import (e.g., use "import type { AnyHandler } ..." instead of the value import)
while leaving other imports in the file unchanged and ensuring all references to
AnyHandler remain as type annotations.
src/core/experimental/sources/network-source.ts (1)

14-17: The as any cast bypasses type safety in the super call.

The spread with as any suggests a mismatch between TypedEvent constructor expectations and the arguments being passed. This could mask type errors if TypedEvent's API changes.

Consider aligning the constructor call with TypedEvent's expected signature or documenting why the cast is necessary.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/sources/network-source.ts` around lines 14 - 17, The
constructor is bypassing type safety by using "as any" in the super call; update
the NetworkSource constructor to call TypedEvent with the correct typed
arguments instead of casting—inspect TypedEvent's constructor signature and
either adjust NetworkSource's class generics (or the constructor parameter
types) so you can call super(type, {}) with proper types, or explicitly
construct the expected event payload type and pass that to super; remove the "as
any" cast from the super(...([type, {}] as any)) call and ensure the constructor
signature and class generics (and the AnyNetworkFrame usage) align with
TypedEvent's expected types so the compiler enforces correctness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@config/scripts/patch-ts.js`:
- Around line 59-66: Add file-level ESLint directives to allow Node globals and
informational logging: at the very top of patch-ts.js add an ESLint env
directive and disable rules that block use of process and console (e.g., /*
eslint-env node */ and /* eslint-disable no-console, no-process-exit */) so the
uses of process.exit and console.log in this script (and other logging blocks)
are permitted by the linter.

In `@package.json`:
- Around line 175-177: The package.json currently only maps the exact import
"#core" which breaks all subpath imports like
"#core/experimental/handlers-controller" and "#core/utils/executeHandlers";
update the "imports" field to add a wildcard subpath mapping for "#core/*"
pointing to the source files (e.g., map "#core/*" -> "./src/core/*") while
keeping the existing "#core" entry so both exact and subpath imports resolve
correctly.

In `@src/browser/index.ts`:
- Line 1: The export in this module currently uses a type-only modifier for
SetupWorkerApi which breaks consumers that import it as a value; update the
export to re-export SetupWorkerApi as a regular export alongside setupWorker
(i.e., remove the "type" modifier from the export of SetupWorkerApi in the file
where setupWorker and SetupWorkerApi are re-exported), or if you intend strict
type-only semantics instead, update all consumer imports to use "import type {
SetupWorkerApi }" — target the export line that mentions setupWorker and
SetupWorkerApi and remove the "type" keyword so existing value-style imports
continue to work.

In `@src/browser/setup-worker.ts`:
- Around line 83-88: The early exit currently returns undefined directly which
violates the StartReturnType (Promise<ServiceWorkerRegistration | undefined>);
change the branch inside the start function so that when isStarted is true it
returns a resolved Promise (e.g., return Promise.resolve(undefined)) instead of
returning undefined, referencing the isStarted check and devUtils.warn in
setup-worker.ts to locate the code and preserve the declared return type.
- Around line 127-135: The stop() implementation fails to reset the isStarted
flag and ignores rejections from network.disable(); update stop() to set
isStarted = false once network.disable() completes (or immediately before/after
posting the 'msw/worker:stop' message) so start() can run again, and attach a
.catch handler (or use try/catch if async) to network.disable() to log or handle
errors instead of leaving the promise unhandled; ensure these changes reference
the existing stop(), isStarted, start(), and network.disable() symbols and
preserve the window.postMessage call.

In `@src/browser/sources/service-worker-source.ts`:
- Around line 69-71: The channel currently captures the original workerPromise
in the constructor (this.#channel = new WorkerChannel({ worker:
this.workerPromise.then(([worker]) => worker) })) so stop()/disable() that
replaces workerPromise leaves the old worker, keepalive timer and beforeunload
listener alive; update stop()/disable() to fully teardown the prior session by
calling a teardown/close on this.#channel (or nulling it), clearing any
keepalive timers, and removing the beforeunload listener, and change how
this.#channel is created/used so it resolves the current workerPromise on each
start (e.g., supply a function/getter or recreate WorkerChannel in start() using
the current this.workerPromise) so start()/stop()/start() cycles can't talk to a
stale worker or leak hooks.
- Around line 244-249: Move the removal of the frame so it always runs before
the opaque-response early return: call this.#frames.delete(request.id)
immediately after retrieving the frame (after const frame =
this.#frames.get(request.id)) and before the if
(response.type?.includes('opaque')) return; check so no stale entries remain for
no-cors/opaque responses; keep the existing this.#frames.get(...) usage and any
subsequent logic that depends on frame.

In `@src/core/experimental/define-network.ts`:
- Around line 166-172: disable() currently calls listenersController.abort() but
listenersController is only assigned in enable(), so calling disable() first
will throw; update disable() to guard against undefined (e.g., if
(listenersController) listenersController.abort()) or initialize
listenersController when the network object is created so it's always defined.
Ensure the symbol listenersController's type/signature is adjusted if needed and
leave the remainder of disable() (the colorlessPromiseAll call over
resolvedOptions.sources.map(source => source.disable())) unchanged.

In `@src/core/experimental/frames/http-frame.ts`:
- Line 143: The request lifecycle currently emits RequestEvent('request:start',
...) but when the handler lookup path calls errorWith() it exits without
emitting a RequestEvent('request:end'), leaking active-request state; update the
handler-lookup/error branch in http-frame.ts to always emit events.emit(new
RequestEvent('request:end', { requestId, request, error })) before returning
(mirror how successful paths emit 'request:end'), ensuring you capture the
thrown/error object and include requestId and request so consumers can clean up;
place this emission immediately before or inside the errorWith() return path so
both the normal and exceptional flows close the lifecycle.
- Around line 215-250: The code currently only emits the
RequestEvent('request:match') after confirming a non-passthrough response, which
hides explicit passthrough handlers; update the logic in the block handling
lookupResult (where you destructure const { response, handler, parsedResult } =
lookupResult) to emit RequestEvent('request:match', { requestId, request }) as
soon as lookupResult is non-null (before the checks for response == null and
isPassthroughResponse(response)), so matched-but-passthrough cases still produce
a 'request:match' event while keeping the existing calls to
storeResponseCookies, passthrough(), and RequestEvent('request:end') intact.
- Line 6: The override of resolve in HttpNetworkFrame narrows the parameter type
to Array<HttpHandler> but getHandlers() returns Array<AnyHandler> and the parent
NetworkFrame declares resolve(handlers: Array<AnyHandler>, ...); update
HttpNetworkFrame.resolve to accept handlers: Array<AnyHandler> (instead of
Array<HttpHandler>) so the signature matches the base class and callers can pass
the output of getHandlers(); similarly widen the parameter in
WebSocketNetworkFrame.resolve from Array<WebSocketHandler> to Array<AnyHandler>,
then adjust any internal type assertions/guards inside those resolve
implementations to narrow handlers to HttpHandler or WebSocketHandler before
using handler-specific properties.

In `@src/core/experimental/frames/websocket-frame.ts`:
- Around line 112-143: The code marks hasMatchingHandlers = true as soon as a
handler.run() yields a handlerConnection, but that should only happen if the
handler actually handled the connection via
handler[kConnect](handlerConnection); change the logic so you call
handler[kConnect](handlerConnection) first and set hasMatchingHandlers = true
only when that call returns truthy (and still ensure removeLogger is disposed
when the connect call returns false); apply the same change to the second
occurrence (the block that mirrors lines 169-179) so only handlers that
successfully run connection listeners flip hasMatchingHandlers.

In `@src/core/experimental/request-utils.test.ts`:
- Around line 28-39: The test for isPassthroughResponse is missing an assertion
and thus always passes; update the test so the expect(...) call on
isPassthroughResponse(new Response(..., { headers: {
[REQUEST_INTENTION_HEADER_NAME]: RequestIntention.passthrough } })) includes a
chained assertion such as .toBe(true) (or .toEqual(true)) so the test actually
verifies the function returns true.

In `@src/core/experimental/sources/interceptor-source.ts`:
- Around line 144-153: The errorWith method incorrectly falls through after
forwarding InternalError to this.#controller.errorWith, causing the same
InternalError to be rethrown; update errorWith (in interceptor-source.ts) so
that after calling this.#controller.errorWith(reason) it immediately returns
(similar to the Response branch) to prevent the extra throw and
double-rejection; make sure you still handle Response via respondWith and only
rethrow for other unknown reasons.

In `@src/core/handlers/RequestHandler.ts`:
- Line 135: The RequestHandler's public property "kind" is mutable and should be
made immutable to prevent accidental reassignment; update the RequestHandler
definition to expose an immutable discriminator (e.g., change the property
declaration on RequestHandler from "public kind = 'request' as const" to a
readonly declaration such as "public readonly kind = 'request' as const" or
implement a read-only getter "public get kind(): 'request' { return 'request'
}") so the handler-kind cannot be reassigned and routing/filtering invariants
remain intact.

In `@test/node/msw-api/setup-server/resetHandlers.node.test.ts`:
- Around line 91-97: The test calls fetch('http://localhost/books', { method:
'POST' }) which doesn't exercise the original handler registered with http.get,
so change the request to use GET (remove or set method to 'GET') so the fetch
targets the same method/path as the original handler; update the fetch
invocation near the hadError variable and keep the surrounding logic that
expects the request to fail after resetHandlers(...) to ensure the test actually
verifies the original GET /books handler was replaced.

In `@test/tsconfig.json`:
- Line 17: Update the tsconfig compilerOptions.types entry to use the package
name "node" instead of "@types/node": locate the compilerOptions.types array
(the "types" key in the tsconfig JSON) and replace the "@types/node" string with
"node" so TypeScript resolves Node globals correctly while keeping
"vitest/globals" as-is.

---

Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 10-11: The CI job named build-20 is configured to run on Node 22
(node-version: 22) while still using cache keys labeled node-20, causing
misleading coverage; either change node-version back to 20 in the build-20 job
and update its cache keys to match, or rename the job to build-22 (and update
the job name/labels and cache key prefixes) and remove or consolidate the
duplicate build-22 job; also apply the same fix pattern to the related jobs
referenced as 25-26 so job names, node-version entries, and cache key prefixes
stay consistent.
- Around line 182-199: The GitHub Actions job "test-e2e" has a mismatched label
vs runtime: the job's name string "test (e2e) (20)" does not match the
configured Node version (actions/setup-node node-version: 22); update either the
name to reflect Node 22 (e.g., change the name value used in the job) or change
the node-version from 22 back to 20 so the label and runtime are consistent
(refer to the "test-e2e" job, the name field "test (e2e) (20)", and the
actions/setup-node with node-version).
- Around line 80-97: The test-unit job currently declares needs: build-20 but
sets actions/setup-node@v4 with node-version: 22, causing a mismatch with the
build that restores a node-20 cache; update the test-unit job (job name:
test-unit) to use node-version: 20 in the actions/setup-node step to match the
build-20 cache, or if you intend to run tests on Node 22 instead, change the
dependency from needs: build-20 to the appropriate build job that ran on Node 22
and ensure the cache key/restore logic aligns with that Node version.
- Around line 116-117: The CI job named test-node-20 is configured to run Node
22 despite its name/label; update the job's Node runtime configuration (e.g.,
the actions/setup-node node-version or the matrix value used by the test-node-20
job) to use node version 20 instead of 22, and make the same correction for the
duplicate occurrence referenced around the other block (the second test-node-20
occurrence at the later lines) so the job name, label, and runtime all
consistently run Node 20.

In `@config/replaceCoreImports.js`:
- Around line 1-10: hasCoreImports uses shared regexes
CORE_ESM_IMPORT_PATTERN/CORE_CJS_IMPORT_PATTERN that were created with the
global flag, so successive test() calls mutate lastIndex and cause flaky
results; fix by ensuring you use fresh regex instances or reset lastIndex before
testing/replacing: update getCoreImportPattern to return a new RegExp based on
the same pattern/flags (or explicitly set getCoreImportPattern(...).lastIndex =
0 before calling test) and apply the same approach in replaceCoreImports so both
hasCoreImports and replaceCoreImports operate on non-stateful regexes.

In `@src/core/ws/utils/attachWebSocketLogger.ts`:
- Around line 85-96: The nested server.addEventListener('message', ...) inside
the 'open' handler is missing the abort signal so it won't be removed on
cleanup; update the inner registration (the listener that calls
logIncomingServerMessage) to pass the same { signal: controller.signal } option
(or use a named handler and call server.removeEventListener(...) on cleanup) so
the message listener is unregistered when controller.signal aborts; locate this
in the block that registers the 'open' event and modify the inner
server.addEventListener call accordingly.

---

Minor comments:
In `@src/core/experimental/frames/websocket-frame.test.ts`:
- Line 182: Fix the typo in the test error message by changing the string passed
to the Error constructor for the variable named exception from "Unhandled
exceptin" to "Unhandled exception" in the websocket-frame.test; locate the
declaration const exception = new Error('Unhandled exceptin') and update the
message text only so the test error message is spelled correctly.

In `@src/core/experimental/handlers-controller.ts`:
- Around line 49-53: currentHandlers() flattens handlers by kind and thus
reorders mixed HTTP/WebSocket registrations; preserve original registration
order by changing state to track insertion sequence and returning handlers in
that sequence: add an ordered list (e.g., state.orderedHandlers) that setup*(),
use(), and resetHandlers() update on add/remove, and modify currentHandlers() to
iterate state.orderedHandlers to collect non-null handlers (instead of
Object.values(...).flat()), or alternately store a registration timestamp on
each handler and sort by it in currentHandlers(); reference currentHandlers(),
setup*(), use(), and resetHandlers() when making these changes.

In `@src/core/experimental/on-unhandled-frame.test.ts`:
- Line 302: Two test titles incorrectly say "HTTP frame" while exercising
TestWebSocketFrame; update the titles to reference WebSocket/TestWebSocketFrame
to avoid confusion. Locate the two it(...) blocks whose current descriptions
begin with "supports printing the default warning in the custom callback for the
HTTP frame" and change them to "supports printing the default warning in the
custom callback for the WebSocket frame (TestWebSocketFrame)" (or similar
wording that includes TestWebSocketFrame), ensuring both occurrences are updated
so CI output correctly identifies the tested frame type.

In `@src/core/experimental/on-unhandled-frame.ts`:
- Around line 18-21: The UnhandledFrameDefaults type and usages declare
warn/error as synchronous but the implementation (printStrategyMessage, which
awaits frame.getUnhandledMessage) returns Promise<void>; update the
UnhandledFrameDefaults type so warn and error are typed as () => Promise<void>,
and adjust any places that construct or call defaults.warn/defaults.error (e.g.,
where defaults are created and where they are invoked in printStrategyMessage
and other call sites around lines 27-45 and 84-91) to handle/await the returned
Promise accordingly (or explicitly return the Promise) so callers can await
these handlers.

In `@src/core/experimental/request-utils.ts`:
- Around line 22-38: The accept-token removal regex in
deleteRequestPassthroughHeader doesn't handle msw/passthrough when it's the
first value; update the replacement to remove the token plus any surrounding
commas/whitespace regardless of position by using a regex like
/(?:^|\s*,\s*)msw\/passthrough(?:\s*,\s*|$)/ for the replace in
deleteRequestPassthroughHeader (i.e., change how nextAcceptHeader is computed),
then normalize the resulting header string by trimming and collapsing any
leftover leading/trailing commas/extra whitespace before setting or deleting the
accept header.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 250-267: The test-native job currently sets node-version: 22 but
declares needs: build-20, causing a Node version mismatch; update the
test-native job (look for the test-native job block and its node-version key) to
align with the intended strategy—either change node-version to 20 to match
build-20 or update the dependent build job and any other jobs to use Node 22
consistently—ensure the node-version value is consistent across jobs that share
the same build dependency.
- Around line 207-224: The test-browser job now uses Node 22 while it depends on
build-20 and cache keys still reference node-20; update the test-browser job
(job name: test-browser) to align Node version and cache with the build job by
setting uses: actions/setup-node@v4 with node-version matching the build (e.g.,
20) and ensure the pnpm cache key or related references that mention node-20 are
consistent, or conversely update build-20 and its cache keys to Node 22 if you
intend to move both jobs to Node 22 so both job names and cache keys remain in
sync (refer to job names test-browser and build-20 and the cache/key entries).

In `@src/browser/glossary.ts`:
- Line 5: The import of AnyHandler in glossary.ts is used only as a type; change
it to a type-only import to clarify intent and aid tree-shaking by replacing the
current import of AnyHandler with a type import (e.g., use "import type {
AnyHandler } ..." instead of the value import) while leaving other imports in
the file unchanged and ensuring all references to AnyHandler remain as type
annotations.

In `@src/core/experimental/compat.ts`:
- Around line 27-34: The synthetic Request created for WebSocket frames (the
ternary branch that checks frame instanceof WebSocketNetworkFrame and constructs
new Request(frame.data.connection.client.url, {...})) does not set an HTTP
method; make it an explicit GET by adding method: 'GET' to the Request init
object so the upgrade request accurately reflects a WebSocket handshake (keep
the existing headers: connection: 'upgrade' and upgrade: 'websocket').

In `@src/core/experimental/request-utils.test.ts`:
- Around line 49-69: Add a new unit test in
src/core/experimental/request-utils.test.ts that verifies
deleteRequestPassthroughHeader correctly removes the "msw/passthrough" token
from a multi-value Accept header while preserving other values; create a Request
with headers.accept set to a comma-separated string like "application/json,
msw/passthrough", call deleteRequestPassthroughHeader(request), and assert that
request.headers.get('accept') equals "application/json" (trimmed of whitespace
as needed) to ensure multi-value handling is covered for the
deleteRequestPassthroughHeader function.

In `@src/core/experimental/setup-api.ts`:
- Around line 30-37: The code creates both emitter and publicEmitter but binds
this.events to emitter so publicEmitter is unused; either remove publicEmitter
entirely or wire events through it: replace this.events = this.emitter with
this.events = this.publicEmitter and then add forwarding so internal emits also
publish to publicEmitter (e.g., in the constructor attach a forwarder from
this.emitter to this.publicEmitter by calling this.emitter.on(event, (...args)
=> this.publicEmitter.emit(event, ...args)) for the relevant event names or by
wrapping emitter.emit to call publicEmitter.emit as well); ensure the cleanup in
this.subscriptions still removes listeners from both this.emitter and
this.publicEmitter.

In `@src/core/experimental/sources/network-source.test.ts`:
- Around line 21-24: Extract the repeated inline test class CustomNetworkSource
into a single shared fixture: create a top-level helper (e.g., a function or
exported test-class) named createCustomNetworkSource or
SharedCustomNetworkSource and replace each inline class declaration (instances
at CustomNetworkSource in the three test sites) with calls to that helper;
update tests that reference enable or instantiate the class to use the shared
helper so the suite doesn't duplicate the same class definition across tests.

In `@src/core/experimental/sources/network-source.ts`:
- Around line 14-17: The constructor is bypassing type safety by using "as any"
in the super call; update the NetworkSource constructor to call TypedEvent with
the correct typed arguments instead of casting—inspect TypedEvent's constructor
signature and either adjust NetworkSource's class generics (or the constructor
parameter types) so you can call super(type, {}) with proper types, or
explicitly construct the expected event payload type and pass that to super;
remove the "as any" cast from the super(...([type, {}] as any)) call and ensure
the constructor signature and class generics (and the AnyNetworkFrame usage)
align with TypedEvent's expected types so the compiler enforces correctness.

In `@src/core/ws/handleWebSocketEvent.ts`:
- Around line 20-24: The current code in handleWebSocketEvent.ts builds handlers
by filtering all handlers via
options.getHandlers().filter(isHandlerKind('websocket')), causing an O(n) scan
per connection; replace that with the kind-indexed lookup by calling
options.getHandlersByKind('websocket') (or the controller method exposed on
options) and assign its result to the handlers variable, removing the
isHandlerKind filter usage; ensure any call-sites expecting an array still work
and that the options object exposes getHandlersByKind so the new call returns
the pre-grouped websocket handlers.

In
`@test/browser/msw-api/setup-worker/scenarios/iframe-isolated-response/iframe-isolated-response.test.ts`:
- Around line 64-66: The two redundant waits using frameOne.waitForFunction(()
=> window.request != null) and frameTwo.waitForFunction(() => window.request !=
null) should be removed because earlier waits already assert typeof
window.request === 'function' (so request cannot be null); delete these two
lines (the calls to waitForFunction that check != null) to avoid duplicate
checks and keep only the prior typeof checks that guarantee a function exists.

In
`@test/browser/msw-api/setup-worker/start/on-unhandled-request/callback-print.test.ts`:
- Line 70: Replace the unconditional page.waitForTimeout(1000) sleeps in
callback-print.test.ts (instances at the call to page.waitForTimeout) with
deterministic waits: wait for the specific console message using
page.waitForEvent('console') filtered by message text, or wait for the network
response with page.waitForResponse() that matches the request URL, or use
page.waitForFunction() to poll a DOM/test-visible state that your assertions
rely on; update both occurrences so assertions run only after these concrete
signals instead of a fixed timeout.

In `@test/browser/msw-api/setup-worker/stop.test.ts`:
- Around line 65-66: The use of expect.soft(response.fromServiceWorker()) is
ambiguous; decide whether the fromServiceWorker() check is critical. If it is
critical, replace expect.soft(response.fromServiceWorker()).toBe(true) with a
regular expect(response.fromServiceWorker()).toBe(true) so the test fails
immediately on mismatch; if the check is intentionally non-blocking, keep
expect.soft but add a brief inline comment above that line explaining why the
assertion is allowed to fail without aborting the test (e.g., "non-fatal check:
service worker presence is optional for this scenario"). Ensure you reference
the same call sites: expect.soft / expect and response.fromServiceWorker(),
leaving the await expect(response.json()).resolves.toEqual({ original: true })
unchanged.

In `@test/node/rest-api/request/body/body-form-data.node.test.ts`:
- Around line 50-54: The status assertion uses expect.soft(res.status) which
allows the test to continue on non-200 responses and can produce misleading
body-failure noise; change the status check to a hard assertion
(expect(res.status).toBe(200)) so the test immediately fails on unexpected HTTP
status and prevents subsequent json assertions from running, updating the
assertion that references res.status in this test file
(body-form-data.node.test.ts) accordingly.

In `@test/support/ws-test-utils.ts`:
- Around line 27-31: The test is using unsafe casts for socket and transport
when instantiating WebSocketServerConnection which masks type mismatches; update
the instantiation around WebSocketServerConnection to either use correctly typed
test doubles/mocks for socket and transport or, if the test environment prevents
that, replace the two "as any" casts with a short inline comment explaining why
the casts are required (e.g., test harness types differ from runtime types) and
reference the variables socket and transport so future maintainers know the
intentional tradeoff; prefer creating minimally-typed stubs matching the
constructor signature over using any when feasible.
- Around line 1-9: The file has duplicate import statements from the same
module; consolidate them into a single import from
'@mswjs/interceptors/WebSocket' that imports WebSocketClientConnection,
WebSocketConnectionData, WebSocketServerConnection, WebSocketTransport, and
WebSocketData together to remove redundancy and keep imports tidy.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

Comment on lines +59 to 66
'Found no .d.ts modules containing the "#core" import, skipping...',
)
return process.exit(0)
}

console.log(
'Found %d module(s) with the "~/core" import, resolving...',
'Found %d module(s) with the "#core" import, resolving...',
typeDefsWithCoreImports.length,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix Node ESLint context for this script to avoid CI failures.

This file uses Node globals (process, console) and informational logs, but current linting flags them as errors. Add explicit Node/script lint directives at the file top so these checks don’t fail.

✅ Proposed fix
+/* eslint-env node */
+/* eslint-disable no-console */
 import fs from 'node:fs'
 import { exec } from 'node:child_process'
 import { promisify } from 'node:util'

Also applies to: 86-90, 145-145

🧰 Tools
🪛 ESLint

[error] 61-61: 'process' is not defined.

(no-undef)


[error] 64-64: 'console' is not defined.

(no-undef)


[error] 64-64: Unexpected console statement. Only these console methods are allowed: warn, error, group, groupCollapsed, groupEnd.

(no-console)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@config/scripts/patch-ts.js` around lines 59 - 66, Add file-level ESLint
directives to allow Node globals and informational logging: at the very top of
patch-ts.js add an ESLint env directive and disable rules that block use of
process and console (e.g., /* eslint-env node */ and /* eslint-disable
no-console, no-process-exit */) so the uses of process.exit and console.log in
this script (and other logging blocks) are permitted by the linter.

Comment on lines +175 to +177
"imports": {
"#core": "./src/core"
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Current package imports map:"
jq '.imports' package.json

echo
echo "Searching for `#core` subpath imports (e.g. `#core/`...):"
rg -nP "(from\\s+['\"]#core/|import\\s*\\(\\s*['\"]#core/)" --type=ts --type=js --glob '!lib/**' --glob '!node_modules/**' || true

Repository: mswjs/msw

Length of output: 4722


Add #core/* mapping—subpath imports currently will fail resolution.

The package.json imports only defines the exact alias #core, but the codebase has 40+ active imports using #core/... subpaths across 15+ files (e.g., #core/experimental/handlers-controller, #core/utils/executeHandlers). These will fail module resolution without the subpath mapping.

✅ Proposed fix
   "imports": {
-    "#core": "./src/core"
+    "#core": "./src/core",
+    "#core/*": "./src/core/*"
   },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"imports": {
"#core": "./src/core"
},
"imports": {
"#core": "./src/core",
"#core/*": "./src/core/*"
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 175 - 177, The package.json currently only maps
the exact import "#core" which breaks all subpath imports like
"#core/experimental/handlers-controller" and "#core/utils/executeHandlers";
update the "imports" field to add a wildcard subpath mapping for "#core/*"
pointing to the source files (e.g., map "#core/*" -> "./src/core/*") while
keeping the existing "#core" entry so both exact and subpath imports resolve
correctly.

export { setupWorker } from './setupWorker/setupWorker'
export type { SetupWorker, StartOptions } from './setupWorker/glossary'
export { SetupWorkerApi } from './setupWorker/setupWorker'
export { setupWorker, type SetupWorkerApi } from './setup-worker'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Check how setup-worker declares SetupWorkerApi:"
ast-grep --pattern $'export class SetupWorkerApi { $$$ }' src/browser/setup-worker.ts || true
ast-grep --pattern $'export interface SetupWorkerApi { $$$ }' src/browser/setup-worker.ts || true
ast-grep --pattern $'export type SetupWorkerApi = $_' src/browser/setup-worker.ts || true

echo
echo "Find in-repo value-like usage of SetupWorkerApi:"
rg -nP '\b(new\s+SetupWorkerApi|instanceof\s+SetupWorkerApi|extends\s+SetupWorkerApi)\b' --type=ts --glob '!lib/**' || true

Repository: mswjs/msw

Length of output: 150


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the actual definition of SetupWorkerApi
echo "=== Search for SetupWorkerApi definition in setup-worker.ts ==="
rg -nA5 'SetupWorkerApi' src/browser/setup-worker.ts || echo "No matches in setup-worker.ts"

echo
echo "=== Check entire src/browser directory for SetupWorkerApi ==="
rg -nA2 'SetupWorkerApi' src/browser/ || echo "No matches in src/browser/"

echo
echo "=== Check for imports of SetupWorkerApi in codebase ==="
rg 'import.*SetupWorkerApi' --type=ts || echo "No imports found"

echo
echo "=== Check package.json exports ==="
cat package.json | grep -A 20 '"exports"' || true

Repository: mswjs/msw

Length of output: 4436


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Search for CHANGELOG or migration notes ==="
rg -i 'type.*setupworkerapi|setupworkerapi.*breaking|setupworkerapi.*export' . --type=md -A 2 || echo "No changelog entries found"

echo
echo "=== Check git history for this line (recent commits) ==="
git log --oneline -n 20 -- src/browser/index.ts 2>/dev/null || echo "Git history not available"

echo
echo "=== Count value vs type imports in tests ==="
echo "Value imports (without type keyword):"
rg 'import\s*\{\s*SetupWorkerApi\s*\}' --type=ts | wc -l

echo "Type imports (with type keyword):"
rg 'import\s*(type\s+)?\{\s*type\s+SetupWorkerApi' --type=ts | wc -l

Repository: mswjs/msw

Length of output: 393


Remove the type modifier to maintain backward compatibility with value imports.

Line 1 enforces type-only imports for SetupWorkerApi, but 16 test files currently use value imports (import { SetupWorkerApi }). This breaks existing consumer code. Since SetupWorkerApi is an interface (type-only at runtime), the type modifier adds strictness but is a breaking change without consumer migration.

Change to: export { setupWorker, SetupWorkerApi } from './setup-worker'

Or if type-only enforcement is intentional, update all consuming code to use import type { SetupWorkerApi }.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/browser/index.ts` at line 1, The export in this module currently uses a
type-only modifier for SetupWorkerApi which breaks consumers that import it as a
value; update the export to re-export SetupWorkerApi as a regular export
alongside setupWorker (i.e., remove the "type" modifier from the export of
SetupWorkerApi in the file where setupWorker and SetupWorkerApi are
re-exported), or if you intend strict type-only semantics instead, update all
consumer imports to use "import type { SetupWorkerApi }" — target the export
line that mentions setupWorker and SetupWorkerApi and remove the "type" keyword
so existing value-style imports continue to work.

Comment on lines +83 to +88
if (isStarted) {
devUtils.warn(
'Found a redundant "worker.start()" call. Note that starting the worker while mocking is already enabled will have no effect. Consider removing this "worker.start()" call.',
)
return
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Returning undefined instead of a Promise breaks the return type contract.

When isStarted is true, the method returns undefined directly, but the declared return type is StartReturnType (Promise<ServiceWorkerRegistration | undefined>). Callers expecting to chain .then() on the result will encounter a runtime error.

Proposed fix
       if (isStarted) {
         devUtils.warn(
           'Found a redundant "worker.start()" call. Note that starting the worker while mocking is already enabled will have no effect. Consider removing this "worker.start()" call.',
         )
-        return
+        return Promise.resolve(undefined)
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (isStarted) {
devUtils.warn(
'Found a redundant "worker.start()" call. Note that starting the worker while mocking is already enabled will have no effect. Consider removing this "worker.start()" call.',
)
return
}
if (isStarted) {
devUtils.warn(
'Found a redundant "worker.start()" call. Note that starting the worker while mocking is already enabled will have no effect. Consider removing this "worker.start()" call.',
)
return Promise.resolve(undefined)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/browser/setup-worker.ts` around lines 83 - 88, The early exit currently
returns undefined directly which violates the StartReturnType
(Promise<ServiceWorkerRegistration | undefined>); change the branch inside the
start function so that when isStarted is true it returns a resolved Promise
(e.g., return Promise.resolve(undefined)) instead of returning undefined,
referencing the isStarted check and devUtils.warn in setup-worker.ts to locate
the code and preserve the declared return type.

Comment on lines +127 to +135
stop() {
if (!isStarted) {
return
}

network.disable().then(() => {
window.postMessage({ type: 'msw/worker:stop' })
})
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

stop() doesn't reset isStarted, preventing valid restart.

After calling stop(), the isStarted flag remains true. A subsequent start() call will incorrectly warn about redundant start and return early without re-enabling the network.

Additionally, the promise rejection from network.disable() is unhandled.

Proposed fix
     stop() {
       if (!isStarted) {
         return
       }
 
-      network.disable().then(() => {
+      isStarted = false
+      network.disable().then(() => {
         window.postMessage({ type: 'msw/worker:stop' })
+      }).catch((error) => {
+        devUtils.warn('Failed to stop the worker: %s', error)
       })
     },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/browser/setup-worker.ts` around lines 127 - 135, The stop()
implementation fails to reset the isStarted flag and ignores rejections from
network.disable(); update stop() to set isStarted = false once network.disable()
completes (or immediately before/after posting the 'msw/worker:stop' message) so
start() can run again, and attach a .catch handler (or use try/catch if async)
to network.disable() to log or handle errors instead of leaving the promise
unhandled; ensure these changes reference the existing stop(), isStarted,
start(), and network.disable() symbols and preserve the window.postMessage call.

Comment on lines +144 to +153
public errorWith(reason?: unknown): void {
if (reason instanceof Response) {
return this.respondWith(reason)
}

if (reason instanceof InternalError) {
this.#controller.errorWith(reason)
}

throw reason
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Return after forwarding InternalError.

Line 150 already hands the failure to the interceptor. Falling through to Line 153 rethrows the same InternalError, so the "error" path can surface as an extra rejection.

Suggested fix
   public errorWith(reason?: unknown): void {
     if (reason instanceof Response) {
       return this.respondWith(reason)
     }
 
     if (reason instanceof InternalError) {
-      this.#controller.errorWith(reason)
+      return this.#controller.errorWith(reason)
     }
 
     throw reason
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public errorWith(reason?: unknown): void {
if (reason instanceof Response) {
return this.respondWith(reason)
}
if (reason instanceof InternalError) {
this.#controller.errorWith(reason)
}
throw reason
public errorWith(reason?: unknown): void {
if (reason instanceof Response) {
return this.respondWith(reason)
}
if (reason instanceof InternalError) {
return this.#controller.errorWith(reason)
}
throw reason
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/experimental/sources/interceptor-source.ts` around lines 144 - 153,
The errorWith method incorrectly falls through after forwarding InternalError to
this.#controller.errorWith, causing the same InternalError to be rethrown;
update errorWith (in interceptor-source.ts) so that after calling
this.#controller.errorWith(reason) it immediately returns (similar to the
Response branch) to prevent the extra throw and double-rejection; make sure you
still handle Response via respondWith and only rethrow for other unknown
reasons.

Comment on lines +91 to +97
const hadError = await fetch('http://localhost/books', {
method: 'POST',
}).then(
() => expect.fail('Request must not succeed'),
() => true,
)
expect(hadError).toBe(true)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use GET /books here; POST never exercises the removed handler.

The original /books handler is registered with http.get(...), so this assertion passes even if resetHandlers(...) fails to replace it. Request the same method/path to actually prove the initial handler is gone.

🐛 Proposed fix
-    const hadError = await fetch('http://localhost/books', {
-      method: 'POST',
-    }).then(
+    const hadError = await fetch('http://localhost/books').then(
       () => expect.fail('Request must not succeed'),
       () => true,
     )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const hadError = await fetch('http://localhost/books', {
method: 'POST',
}).then(
() => expect.fail('Request must not succeed'),
() => true,
)
expect(hadError).toBe(true)
const hadError = await fetch('http://localhost/books').then(
() => expect.fail('Request must not succeed'),
() => true,
)
expect(hadError).toBe(true)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/node/msw-api/setup-server/resetHandlers.node.test.ts` around lines 91 -
97, The test calls fetch('http://localhost/books', { method: 'POST' }) which
doesn't exercise the original handler registered with http.get, so change the
request to use GET (remove or set method to 'GET') so the fetch targets the same
method/path as the original handler; update the fetch invocation near the
hadError variable and keep the surrounding logic that expects the request to
fail after resetHandlers(...) to ensure the test actually verifies the original
GET /books handler was replaced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants